Preserve inline table key order in item() (#546)#552
Conversation
`item()` sorted dict-valued keys last in the list/tuple branch even under the
default ``sort_keys=False``, because the sort key kept ``isinstance(i[1],
dict)`` in the tuple unconditionally (unlike the dict branch). For an inline
table this silently reordered keys, e.g.
``item([[{"a": {"x": 1}, "b": 2}]])`` produced ``[[{b = 2, a = {x = 1}}]]``.
Only apply the dicts-last ordering to arrays of tables (which render as
``[[table]]`` headers, where a dict-valued key would otherwise capture the
following keys). Inline tables cannot capture, so their insertion order is now
preserved unless sorting is explicitly requested.
Closes python-poetry#546
| # Inline tables cannot capture, so preserve insertion order unless | ||
| # explicitly sorting (matching the dict branch above). See #546. | ||
| def _sort_key(i: tuple[Any, Any]) -> Any: | ||
| return (isinstance(i[1], dict), i[0]) if _sort_keys else 1 |
There was a problem hiding this comment.
why are you keeping dictionaries last in this branch?
There was a problem hiding this comment.
Only for consistency with the sibling dict branch above (line ~146), which uses this exact sort key — (isinstance(i[1], dict), i[0]) if _sort_keys else 1 — and also constructs InlineTables (when _parent is an Array/InlineTable). With this form, sort_keys=True produces the same ordering for an inline table regardless of whether it was built via the dict branch or the list branch.
I kept the scope of this PR to the sort_keys=False default (the #546 bug). If you'd rather have sorted inline tables be purely alphabetical (no dicts-last), I'm happy to change it — but then the dict branch should probably change too, so the two paths don't diverge. Let me know which you'd prefer.
There was a problem hiding this comment.
The sibling branch needs to put dictionaries last so as to avoid unwanted scalar capture.
The whole point of your fix is that the inline-table branch has no such requirement, so it seems weird that you would interpret sort_keys to mean "sort keys, but still put dictionaries last"
2 participants
Summary
Closes #546.
item()reordered the keys of an inline table under the defaultsort_keys=False:Root cause
In the
list/tuplebranch ofitem(), the sort key wasso
isinstance(i[1], dict)stayed in the tuple unconditionally, forcing dict-valued keys last even when not sorting. The siblingdictbranch has the correct form ((isinstance(i[1], dict), i[0]) if _sort_keys else 1).Fix
Keep the dicts-last ordering only for arrays of tables (
Tableconstructor), which render as[[table]]headers where a dict-valued key would otherwise capture the following keys — this is why simply removing the dicts-last ordering (PR #547) was not acceptable. Inline tables cannot capture, so their insertion order is preserved unlesssort_keys=Trueis passed (matching thedictbranch).Verified:
[[{a = {x = 1}, b = 2}]](order preserved)_sort_keys=True:[[{b = 2, a = {x = 1}}]](still sorted, dicts-last)b = 2still emitted before the[a]sub-table headerTests
Added
test_item_inline_table_preserves_key_ordercovering all three cases. It fails onmasterand passes with this change. Full suite passes (356 passed);ruff check/ruff formatclean; added a CHANGELOG entry.